GH-50508: [C++] Support scalar values in AppendScalars - #50584
Conversation
|
|
kou
changed the title
|
Could you use our PR template and fix CI failures? |
Done. I updated the PR description using the project template and applied the required clang-format changes. The latest commit has been pushed, and the CI workflows are currently awaiting maintainer approval. |
|
Approved. FYI: You can check CI results on your fork by enabling GitHub Actions on your fork. |
It seems that the PR description isn't updated yet... |
| UpdateDimensions(committed_logical_length_, 0); | ||
| return Status::OK(); | ||
| } | ||
|
|
There was a problem hiding this comment.
OTOH, let's please keep an empty line between function and method definitions.
|
I've separately created #50596 to refactor the |
github-actions
Bot
added
awaiting committer review
Done. I addressed the review comments. |
| ASSERT_OK_AND_ASSIGN(logical_run_ends, ree_slice->LogicalRunEnds(pool)); | ||
| ASSERT_ARRAYS_EQUAL(*logical_run_ends, *expected_run_ends); | ||
| } | ||
|
|
There was a problem hiding this comment.
Can you restore this empty line?
|
Hi @pitrou , I have addressed the latest review comments, and all CI checks are passing. The PR is ready for review. Thanks! |
pitrou
left a comment
pitrou
left a comment
There was a problem hiding this comment.
+1, thank you @Moemenmohamed24
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 9c37a70. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them. |
3 participants
Rationale for this change
RunEndEncodedBuilder::AppendScalaraccepts regular scalar values, such asfloat32directly. However,RunEndEncodedBuilder::AppendScalarsonly handledRunEndEncodedScalarvalues.This made the two APIs inconsistent. This PR updates
AppendScalarsto support regular scalar values consistently withAppendScalar.What changes are included in this PR?
RunEndEncodedBuilder::AppendScalarsto process each scalar throughAppendScalar.RunEndEncodedScalarvalues.Are these changes tested?
Yes. The relevant Run-End Encoded tests pass successfully.
Are there any user-facing changes?
Yes.
RunEndEncodedBuilder::AppendScalarsnow accepts regular scalar values, such asfloat32, consistently withAppendScalar.Breaking changes
No
Critical Fix
No.